-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Y24-190-3 Use API v2 TagLayout #1901
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop-Y24-190 #1901 +/- ##
================================================
Coverage 77.97% 77.97%
================================================
Files 459 460 +1
Lines 17685 17686 +1
Branches 229 229
================================================
+ Hits 13789 13790 +1
Misses 3894 3894
Partials 2 2
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks sensible.
@@ -232,17 +233,18 @@ def expect_state_change_creation | |||
let(:tag_plate_state) { 'exhausted' } | |||
|
|||
it 'creates a tagged plate' do | |||
# This one will be VERY different |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Intentional comment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well it came from removed line 241 below and I'm making the same expectation, just via API V2, so hopefully it's intentional.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah just not sure what the difference is that the comment is suggesting.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know. I didn't want to try to clean up existing comments and I assumed it was made for a reason. It's very easy to put in comments that aren't useful in the future 😄
Closes #1818
Changes proposed in this pull request
Instructions for Reviewers
[All PRs] - Confirm PR template filled
[Feature Branches] - Review code